Skip to content

src: add Object::TypeTag, Object::CheckTypeTag#1261

Closed
KevinEady wants to merge 4 commits intonodejs:mainfrom
KevinEady:add-type-tag-methods
Closed

src: add Object::TypeTag, Object::CheckTypeTag#1261
KevinEady wants to merge 4 commits intonodejs:mainfrom
KevinEady:add-type-tag-methods

Conversation

@KevinEady
Copy link
Contributor

@KevinEady KevinEady commented Jan 2, 2023

Adds support for napi_type_tag_object via Object::TypeTag and napi_check_object_type_tag via Object::CheckTypeTag.

Closes: #1260

@KevinEady
Copy link
Contributor Author

KevinEady commented Jan 2, 2023 •
edited
Loading

Looks like the test has different outcomes depending on Node version, which I can reproduce locally. The test (cpp, js) is similar to the one found in node core (cpp, js).

It looks like there was an issue in type tag checking nodejs/node#43786 fixed by nodejs/node#43788 ... Was this backported to other Node versions?

EDIT: I just changed the test to not use the bugged checks if node version < 18.

v14: Failure ❌

➜  node-addon-api git:(add-type-tag-methods) ✗ nvm use v14                        
Now using node v14.21.2 (npm v6.14.17)
➜  node-addon-api git:(add-type-tag-methods) ✗ node test/object/object_type_tag.js
(node:13959) UnhandledPromiseRejectionWarning: AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

false !== true

    at test (/Users/kevineady/Documents/Projects/node-addon-api/test/object/object_type_tag.js:17:10)
    at Object.exports.runTest (/Users/kevineady/Documents/Projects/node-addon-api/test/common/index.js:122:27)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:13959) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:13959) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

v16: Failure ❌

➜  node-addon-api git:(add-type-tag-methods) ✗ nvm use v16                        
Now using node v16.15.1 (npm v8.11.0)
➜  node-addon-api git:(add-type-tag-methods) ✗ node test/object/object_type_tag.js
node:internal/process/promises:279
            triggerUncaughtException(err, true /* fromPromise */);
            ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

false !== true

    at test (/Users/kevineady/Documents/Projects/node-addon-api/test/object/object_type_tag.js:17:10)
    at Object.exports.runTest (/Users/kevineady/Documents/Projects/node-addon-api/test/common/index.js:122:27) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: 'strictEqual'
}

v18: Success ✅

➜  node-addon-api git:(add-type-tag-methods) ✗ nvm use v18                        
Now using node v18.12.1 (npm v8.19.2)
➜  node-addon-api git:(add-type-tag-methods) ✗ node test/object/object_type_tag.js

v19: Success ✅

➜  node-addon-api git:(add-type-tag-methods) ✗ nvm use v19    
Now using node v19.3.0 (npm v9.2.0)
➜  node-addon-api git:(add-type-tag-methods) ✗ node test/object/object_type_tag.js

mhdawson
mhdawson approved these changes Jan 9, 2023
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

mhdawson pushed a commit that referenced this pull request Jan 13, 2023
@KevinEady @mhdawson
src: add Object::TypeTag, Object::CheckTypeTag
b11e4de 
PR-URL: #1261
Reviewed-By: Michael Dawson <midawson@redhat.com
@mhdawson
Copy link
Member

mhdawson commented Jan 13, 2023

Landed as b11e4de

@mhdawson mhdawson closed this Jan 13, 2023
@audetto audetto mentioned this pull request Feb 27, 2023
Closed
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
src: add Object::TypeTag, Object::CheckTypeTag
1251e70 
PR-URL: nodejs/node-addon-api#1261
Reviewed-By: Michael Dawson <midawson@redhat.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhdawson mhdawson mhdawson approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing napi_type_tag_object?

2 participants

@KevinEady @mhdawson